Skip to content

Conversation

@jasugun
Copy link
Contributor

@jasugun jasugun commented Dec 2, 2024

Use getattr method with default instead, defaults visible
env.has_attribute() is not working whith boolean args when False, arg is not None returns True in that case.

@jasugun jasugun requested review from ftith and tdesveaux December 2, 2024 11:00
Use getattr method with default instead, defaults visible
@jasugun jasugun force-pushed the lca/unreal/fix_nop4_args branch from dda8978 to 3f41dc5 Compare December 2, 2024 13:04
p4_args_for_commandlet.append('-P4Passwd=%s' % env.p4pass)
if env.has_attribute('p4client'):
p4_args_for_commandlet.append('-P4Client=%s' % env.p4client)
if getattr(env, 'p4port', None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why replace env.has_attribute?

Copy link
Contributor Author

@jasugun jasugun Dec 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because out of the box, short python achieves the same result as the helper, right?
The helper should be named attribute_is_not_none btw, it's misealding and was most likely the cause of the bug with bools.
@tdesveaux do you see any problem with taht?
If so I'll stick to changing the bools only.

@jasugun jasugun requested a review from tdesveaux December 3, 2024 17:07
@tdesveaux tdesveaux deleted the branch main April 4, 2025 09:12
@tdesveaux tdesveaux closed this Apr 4, 2025
@tdesveaux
Copy link
Contributor

Wrongly closed by dev branch deletion

@tdesveaux tdesveaux reopened this Apr 4, 2025
@tdesveaux tdesveaux changed the base branch from dev to main April 4, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants